Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K8SPG-407 Allow perconapgclusters/finalizers in role #229

Merged
merged 5 commits into from
Jul 25, 2023

Conversation

thomaspetit
Copy link
Contributor

@thomaspetit thomaspetit commented Jul 12, 2023

Allow perconapgclusters/finalizers to ensure block deletion permissions are granted. This will otherwise cause a apply failure when the operator tries to instantiate a CR on OpenShift 4.10+ (confirmed and tested with this version).

The operator will complain with following error:

time="2023-07-12T11:47:18Z" level=error msg="Reconciler error" PerconaPGCluster=pgportal/pgportal-postgresql controller=perconapgcluster controllerGroup=pgv2.percona.com controllerKind=PerconaPGCluster error="update/create PostgresCluster: postgresclusters.postgres-operator.crunchydata.com \"pgportal-postgresql\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>" file="percona/controller/pgcluster/controller.go:243" func="pgcluster.(*PGClusterReconciler).Reconcile" name=pgportal-postgresql namespace=pgportal reconcileID=9d167f6c-d211-426e-8884-7c5159da08ae version=

This is the interesting error you see:
cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on

Allow perconapgclusters/finalizers to ensure block deletion permissions are granted. This will otherwise cause a apply failure when the operator tries to instantiate a CR on OpenShift 4.10+ (tested)
@it-percona-cla
Copy link

it-percona-cla commented Jul 12, 2023

CLA assistant check
All committers have signed the CLA.

@thomaspetit
Copy link
Contributor Author

@tplavcic Can you have a look at this?

@tplavcic
Copy link
Member

@thomaspetit Need to discuss it within team so somebody will have a look. To me this doesn't look like helm chart specific but rather general issue with the current release.

@thomaspetit
Copy link
Contributor Author

Can you tell me why this is not chart specific, the role is defined in the chart as it is? Is this role generated automatically from somewhere? If you want I can probably make the change upstream if I know where to look :)

@egegunes
Copy link
Contributor

@thomaspetit what @tplavcic is talking about is the role in the operator repo. We should also fix it there, if you're up to it you can create a PR but you don't need to, we'll do in the next release anyway.

egegunes
egegunes previously approved these changes Jul 17, 2023
nmarukovich
nmarukovich previously approved these changes Jul 17, 2023
@@ -2,7 +2,7 @@ apiVersion: v2
name: pg-operator
description: 'A Helm chart to deploy the v2 version of Percona Operator for PostgreSQL.'
type: application
version: 2.2.0
version: 2.2.1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomaspetit please bump the version to 2.2.2 because #230 was merged :( P.S. thank you for PR!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I am not sure that all these create/get/list/patch/update/watch verbs are needed for 'perconapgclusters/finalizers'. Only an update can be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Sorry for the delay ;)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomaspetit Please bump it one more time to 2.2.3 and we will merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me why? Currently the version is 2.2.1 on main while this new version will be 2.2.2. Am I missing something? :-) Sorry for the question..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, It was my mistake. :) Thank you for your contribution.

@hors hors changed the title Allow perconapgclusters/finalizers in role K8SPG-407 Allow perconapgclusters/finalizers in role Jul 17, 2023
@thomaspetit thomaspetit dismissed stale reviews from nmarukovich and egegunes via 68a685f July 22, 2023 19:08
@hors hors merged commit d76a999 into percona:main Jul 25, 2023
1 check passed
@thomaspetit thomaspetit deleted the add-perconapgclusters-finalizers-role branch July 29, 2023 21:00
@thomaspetit thomaspetit restored the add-perconapgclusters-finalizers-role branch July 29, 2023 21:00
@thomaspetit thomaspetit deleted the add-perconapgclusters-finalizers-role branch July 29, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants